repo: Fix static delta progress display

There were a few bugs here.

- We need to keep track of the size of the delta parts we've already processed,
  in order to make progress reliable at all in the face of interruptions.  Add
  a new `fetched-delta-part-size` async progress variable for this.
- The total before disregarded what we'd already downloaded, which was confusing.
  Now, a progress percentage is `fetched/total`.
- Correctly handle "unknown bytes/sec" in the progress display.

However, to be fully correct we need to show the fallback objects too. That
would require tracking in the pull code when we fetch an object as a fallback
versus "normally". This would be simpler really if we could assume in a run we
were *only* processing a delta, but currently we don't do that.

Related: https://github.com/ostreedev/ostree/issues/475

Closes: #678
Approved by: giuseppe

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 0e5128d9ee978e4cb2d64c1e34c6e0ad0f65fb43..129e25daabff5ad0e21abda680ca3cdf890b6f0d 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -91,6 +91,8 @@ typedef struct {
   guint             n_outstanding_deltapart_fetches;
   guint             n_outstanding_deltapart_write_requests;
   guint             n_total_deltaparts;
+  guint             n_total_delta_fallbacks;
+  guint64           fetched_deltapart_size; /* How much of the delta we have now */
   guint64           total_deltapart_size;
   guint64           total_deltapart_usize;
   gint              n_requested_metadata;
@@ -220,6 +222,10 @@ update_progress (gpointer user_data)
                                   pull_data->n_fetched_deltaparts);
   ostree_async_progress_set_uint (pull_data->progress, "total-delta-parts",
                                   pull_data->n_total_deltaparts);
+  ostree_async_progress_set_uint (pull_data->progress, "total-delta-fallbacks",
+                                  pull_data->n_total_delta_fallbacks);
+  ostree_async_progress_set_uint64 (pull_data->progress, "fetched-delta-part-size",
+                                    pull_data->fetched_deltapart_size);
   ostree_async_progress_set_uint64 (pull_data->progress, "total-delta-part-size",
                                     pull_data->total_deltapart_size);
   ostree_async_progress_set_uint64 (pull_data->progress, "total-delta-part-usize",
@@ -1590,15 +1596,10 @@ process_one_static_delta_fallback (OtPullData   *pull_data,
   compressed_size = maybe_swap_endian_u64 (delta_byteswap, compressed_size);
   uncompressed_size = maybe_swap_endian_u64 (delta_byteswap, uncompressed_size);
 
+  pull_data->n_total_delta_fallbacks += 1;
   pull_data->total_deltapart_size += compressed_size;
   pull_data->total_deltapart_usize += uncompressed_size;
 
-  if (pull_data->dry_run)
-    {
-      ret = TRUE;
-      goto out;
-    }
-
   objtype = (OstreeObjectType)objtype_y;
   checksum = ostree_checksum_from_bytes_v (csum_v);
 
@@ -1607,6 +1608,15 @@ process_one_static_delta_fallback (OtPullData   *pull_data,
                                cancellable, error))
     goto out;
 
+  if (is_stored)
+    pull_data->fetched_deltapart_size += compressed_size;
+
+  if (pull_data->dry_run)
+    {
+      ret = TRUE;
+      goto out;
+    }
+
   if (!is_stored)
     { 
       if (OSTREE_OBJECT_TYPE_IS_META (objtype))
@@ -1779,11 +1789,15 @@ process_one_static_delta (OtPullData   *pull_data,
                                                             cancellable, error))
         goto out;
 
+      pull_data->total_deltapart_size += size;
+      pull_data->total_deltapart_usize += usize;
+
       if (have_all)
         {
           g_debug ("Have all objects from static delta %s-%s part %u",
                    from_revision ? from_revision : "empty", to_revision,
                    i);
+          pull_data->fetched_deltapart_size += size;
           pull_data->n_fetched_deltaparts++;
           continue;
         }
@@ -1797,9 +1811,6 @@ process_one_static_delta (OtPullData   *pull_data,
           inline_part_bytes = g_variant_get_data_as_bytes (part_datav);
       }
 
-      pull_data->total_deltapart_size += size;
-      pull_data->total_deltapart_usize += usize;
-
       if (pull_data->dry_run)
         continue;
       

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index e7cbb16d89696d0984f3d7954911c7cb3bf9abbb..98abd9265ad066fa7b99ce57dde4742f4bf24bee 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3848,8 +3848,7 @@ _formatted_time_remaining_from_seconds (guint64 seconds_remaining)
   if (minutes_remaining)
     g_string_append_printf (description, "%" G_GUINT64_FORMAT " minutes ", minutes_remaining % 60);
 
-  if (seconds_remaining)
-    g_string_append_printf (description, "%" G_GUINT64_FORMAT " seconds ", seconds_remaining % 60);
+  g_string_append_printf (description, "%" G_GUINT64_FORMAT " seconds ", seconds_remaining % 60);
 
   return g_string_free (description, FALSE);
 }
@@ -3913,33 +3912,49 @@ ostree_repo_pull_default_console_progress_changed (OstreeAsyncProgress *progress
       g_autofree char *formatted_bytes_transferred =
         g_format_size_full (bytes_transferred, 0);
       g_autofree char *formatted_bytes_sec = NULL;
-      g_autofree char *formatted_est_time_remaining = NULL;
+      guint64 bytes_sec;
 
       /* Ignore the first second, or when we haven't transferred any
        * data, since those could cause divide by zero below.
        */
       if ((current_time - start_time) < G_USEC_PER_SEC || bytes_transferred == 0)
         {
+          bytes_sec = 0;
           formatted_bytes_sec = g_strdup ("-");
-          formatted_est_time_remaining = g_strdup ("- ");
         }
       else
         {
-          guint64 bytes_sec = bytes_transferred / ((current_time - start_time) / G_USEC_PER_SEC);
-          guint64 est_time_remaining =  (total_delta_part_size - bytes_transferred) / bytes_sec;
+          bytes_sec = bytes_transferred / ((current_time - start_time) / G_USEC_PER_SEC);
           formatted_bytes_sec = g_format_size (bytes_sec);
-          formatted_est_time_remaining = _formatted_time_remaining_from_seconds (est_time_remaining);
         }
 
+      /* Are we doing deltas?  If so, we can be more accurate */
       if (total_delta_parts > 0)
         {
+          guint64 fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
+          g_autofree char *formatted_fetched =
+            g_format_size (fetched_delta_part_size);
           g_autofree char *formatted_total =
             g_format_size (total_delta_part_size);
-          /* No space between %s and remaining, since formatted_est_time_remaining has a trailing space */
-          g_string_append_printf (buf, "Receiving delta parts: %u/%u %s/s %s/%s %sremaining",
-                                  fetched_delta_parts, total_delta_parts,
-                                  formatted_bytes_sec, formatted_bytes_transferred,
-                                  formatted_total, formatted_est_time_remaining);
+
+          if (bytes_sec > 0)
+            {
+              /* MAX(0, value) here just to be defensive */
+              guint64 est_time_remaining = MAX(0, (total_delta_part_size - fetched_delta_part_size)) / bytes_sec;
+              g_autofree char *formatted_est_time_remaining = _formatted_time_remaining_from_seconds (est_time_remaining);
+              /* No space between %s and remaining, since formatted_est_time_remaining has a trailing space */
+              g_string_append_printf (buf, "Receiving delta parts: %u/%u %s/%s %s/s %sremaining",
+                                      fetched_delta_parts, total_delta_parts,
+                                      formatted_fetched, formatted_total,
+                                      formatted_bytes_sec,
+                                      formatted_est_time_remaining);
+            }
+          else
+            {
+              g_string_append_printf (buf, "Receiving delta parts: %u/%u %s/%s",
+                                      fetched_delta_parts, total_delta_parts,
+                                      formatted_fetched, formatted_total);
+            }
         }
       else if (scanning || outstanding_metadata_fetches)
         {

diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index f370ca018ace3c07ddb4279170ee7b1a66560b85..7df48002445cd3a9e73feefe52a9b91b5532d60a 100644 (file)
--- a/src/ostree/ot-builtin-pull.c
+++ b/src/ostree/ot-builtin-pull.c
@@ -80,7 +80,7 @@ dry_run_console_progress_changed (OstreeAsyncProgress *progress,
                                   gpointer             user_data)
 {
   guint fetched_delta_parts, total_delta_parts;
-  guint64 total_delta_part_size, total_delta_part_usize;
+  guint64 fetched_delta_part_size, total_delta_part_size, total_delta_part_usize;
   GString *buf;
 
   g_assert (!printed_console_progress);
@@ -88,19 +88,23 @@ dry_run_console_progress_changed (OstreeAsyncProgress *progress,
 
   fetched_delta_parts = ostree_async_progress_get_uint (progress, "fetched-delta-parts");
   total_delta_parts = ostree_async_progress_get_uint (progress, "total-delta-parts");
+  fetched_delta_part_size = ostree_async_progress_get_uint64 (progress, "fetched-delta-part-size");
   total_delta_part_size = ostree_async_progress_get_uint64 (progress, "total-delta-part-size");
   total_delta_part_usize = ostree_async_progress_get_uint64 (progress, "total-delta-part-usize");
 
   buf = g_string_new ("");
 
-  { g_autofree char *formatted_size =
+  { g_autofree char *formatted_fetched =
+      g_format_size (fetched_delta_part_size);
+    g_autofree char *formatted_size =
       g_format_size (total_delta_part_size);
     g_autofree char *formatted_usize =
       g_format_size (total_delta_part_usize);
 
-    g_string_append_printf (buf, "Delta update: %u/%u parts, %s to transfer, %s uncompressed",
+    g_string_append_printf (buf, "Delta update: %u/%u parts, %s/%s, %s total uncompressed",
                             fetched_delta_parts, total_delta_parts,
-                            formatted_size, formatted_usize);
+                            formatted_fetched, formatted_size,
+                            formatted_usize);
   }
   g_print ("%s\n", buf->str);
   g_string_free (buf, TRUE);